Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the "safe_name" attribute of PackageFile for backwards compatibility #745

Merged
merged 2 commits into from
Mar 17, 2021

Conversation

pablogsal
Copy link
Contributor

Commit 0bd26af introduced a regression
that was causing some namespace packages with dots in them fail to
upload to PyPI. The reason is because
packaging.utils.canonicalize_name() is not equivalent to
pkg_resources.safe_name() as the former transforms the "." into "-",
while the later only does it for non-alphanumeric/. characters.

Closes: #743

@pablogsal pablogsal force-pushed the issue-743 branch 2 times, most recently from e5641cf to 57a19f3 Compare March 16, 2021 01:19
@pablogsal
Copy link
Contributor Author

CC: @bhrutledge @jaraco

@pablogsal pablogsal force-pushed the issue-743 branch 3 times, most recently from 0027889 to 92c78cb Compare March 16, 2021 01:28
Commit 0bd26af introduced a regression
that was causing some namespace packages with dots in them fail to
upload to PyPI. The reason is because
`packaging.utils.canonicalize_name()` is not equivalent to
`pkg_resources.safe_name()` as the former transforms the "." into "-",
while the later only does it for non-alphanumeric/. characters.

Closes: pypa#743
Copy link
Contributor

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, test, and changelog! One small suggestion for the docstring, and I'd still like to get sign-off from the Twine maintainer that made the original change.

twine/package.py Outdated Show resolved Hide resolved
Co-authored-by: Brian Rutledge <brian@bhrutledge.com>
Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. This was my original instinct before converting to canonicalize name. Does this change remove the last dependency on packaging?

@pablogsal
Copy link
Contributor Author

Does this change remove the last dependency on packaging?

It seems so. I would recommend doing the removal in a different change to keep things clean but I can push some commit to remove packaging from the install_requires and other places if you wish

@bhrutledge
Copy link
Contributor

Does this change remove the last dependency on packaging?

It seems so. I would recommend doing the removal in a different change to keep things clean but I can push some commit to remove packaging from the install_requires and other places if you wish

Thanks @pablogsal. I'm going to merge this, then follow up with the removal of packaging, then release 3.4.1.

This was referenced Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Namespace packages with dots in them fail to upload to PyPI
3 participants